Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(agents-api): Add some LiteLLM exceptions to the list of non-retryable errors #622

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

HamadaSalhab
Copy link
Contributor

@HamadaSalhab HamadaSalhab commented Oct 9, 2024

Added the following exceptions:

  • litellm.exceptions.NotFoundError.
  • litellm.exceptions.InvalidRequestError.
  • litellm.exceptions.AuthenticationError.
  • litellm.exceptions.ServiceUnavailableError.
  • litellm.exceptions.OpenAIError.
  • litellm.exceptions.APIError.

Important

Add LiteLLM exceptions to non-retryable errors in tasks.py.

  • Exceptions:
    • Added litellm.exceptions.NotFoundError, InvalidRequestError, AuthenticationError, ServiceUnavailableError, OpenAIError, and APIError to NON_RETRYABLE_ERROR_TYPES in tasks.py.

This description was created by Ellipsis for 018360c. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 018360c in 16 seconds

More details
  • Looked at 40 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/common/exceptions/tasks.py:99
  • Draft comment:
    ServiceUnavailableError is typically a retryable error, as it might indicate a temporary issue. Consider if this should be non-retryable.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative, suggesting a reconsideration of the classification of ServiceUnavailableError. However, the file's purpose is to define non-retryable errors, and the inclusion of ServiceUnavailableError seems intentional. The comment does not provide strong evidence that a change is required, as it is based on a typical scenario rather than a definite issue.
    I might be overlooking the possibility that ServiceUnavailableError is commonly considered retryable in other contexts, which could justify the comment. However, without strong evidence or a clear mandate to change the classification, the comment remains speculative.
    The comment lacks strong evidence and is speculative, which does not align with the rules for review comments. It suggests a reconsideration without a definitive need for change.
    The comment should be deleted as it is speculative and does not provide strong evidence for a required code change.

Workflow ID: wflow_jgKYGnUxagzMCe6V


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@HamadaSalhab HamadaSalhab merged commit 4fe49d1 into dev Oct 10, 2024
10 of 14 checks passed
@HamadaSalhab HamadaSalhab deleted the f/litellm-non-retryable-errors branch October 10, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant